Skip to content

Conversation

PWagner1
Copy link
Contributor

@PWagner1 PWagner1 commented Sep 13, 2025

@giduac
Copy link
Contributor

giduac commented Sep 15, 2025

Will be testing during this week

Copy link
Contributor

@giduac giduac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PWagner1

A strange thing is that the InternalPanel is selectable. if you click that area. When the title-bar is clicked the KForm is selected. The latter should be the case when any part of the form is clicked, the form is selected in the properties window.

I cant see any improvement in this branch atm.
Also controls cannot be added, as was the case before.

devenv_hAwOMPRKTp.mp4

Please have this corrected before requesting a review.
Thanks.

@PWagner1
Copy link
Contributor Author

@PWagner1

A strange thing is that the InternalPanel is selectable. if you click that area. When the title-bar is clicked the KForm is selected. The latter should be the case when any part of the form is clicked, the form is selected in the properties window.

I cant see any improvement in this branch atm. Also controls )cannot be added, as was the case before.

devenv_hAwOMPRKTp.mp4
Please have this corrected before requesting a review. Thanks.

@giduac

I don't know why, but it seems to work correctly in some instances but not others...

@giduac
Copy link
Contributor

giduac commented Sep 16, 2025

@PWagner1
A strange thing is that the InternalPanel is selectable. if you click that area. When the title-bar is clicked the KForm is selected. The latter should be the case when any part of the form is clicked, the form is selected in the properties window.
I cant see any improvement in this branch atm. Also controls )cannot be added, as was the case before.
devenv_hAwOMPRKTp.mp4
Please have this corrected before requesting a review. Thanks.

@giduac

I don't know why, but it seems to work correctly in some instances but not others...

Why does the menu interfere with the controls area / internalPanel...

@PWagner1
Copy link
Contributor Author

@PWagner1
A strange thing is that the InternalPanel is selectable. if you click that area. When the title-bar is clicked the KForm is selected. The latter should be the case when any part of the form is clicked, the form is selected in the properties window.
I cant see any improvement in this branch atm. Also controls )cannot be added, as was the case before.
devenv_hAwOMPRKTp.mp4
Please have this corrected before requesting a review. Thanks.

@giduac
I don't know why, but it seems to work correctly in some instances but not others...

Why does the menu interfere with the controls area / internalPanel...

@giduac

That, I don't understand, as it never touches it?

@PWagner1
Copy link
Contributor Author

@giduac

Yesssssssssssssss......... fixed it!!!!!!!!!!!

@giduac
Copy link
Contributor

giduac commented Sep 17, 2025

@giduac

Yesssssssssssssss......... fixed it!!!!!!!!!!!

I'd like to take this a bit further.

  • Why does the themed menu react to (mouse) input that is not in the title-bar?
  • Another approach to look at: can the themed menu be made to not react to input while the form is in design mode. That can make the code in KInternalpanel redundant. And that code will run a lot since the panel is at the core of kform.

@giduac
Copy link
Contributor

giduac commented Sep 17, 2025

@PWagner1

And controls cant be added normally nor can they be moved across the form.
https://github.com/user-attachments/assets/ecaf5425-f95d-446f-9116-dff4c0d25912

@PWagner1
Copy link
Contributor Author

@PWagner1

And controls cant be added normally nor can they be moved across the form. https://github.com/user-attachments/assets/ecaf5425-f95d-446f-9116-dff4c0d25912

@giduac

This is pretty much of a hit and miss...

Screen.Recording.2025-09-17.084932.mp4

@giduac
Copy link
Contributor

giduac commented Sep 18, 2025

@PWagner1
And controls cant be added normally nor can they be moved across the form. https://github.com/user-attachments/assets/ecaf5425-f95d-446f-9116-dff4c0d25912

@giduac

This is pretty much of a hit and miss...

Screen.Recording.2025-09-17.084932.mp4

So disabling worked at designtime, nice.

There have been quite a few edits to try to get this fixed. Which most of those are likely unnecessary. So best is to clean all this out and get it nice and tidy. When going over the code I think there are also some stale code blocks.

After that I'd like to go over the code and have a look at the need for nullability on vars/instances to reduce null checks.
A question mark doesn't look like much but will result in a full null check under the hood. So there's a bit to gain here.

# EDIT:

Or maybe better take a new branch from Alpha to lose all the changes.
And add the part that turns the menu off...

And:

Since this all happens in the form instance the Form.DesignModeproperty can be used directly and don't need this code block.
Several (not all) components/controls have this property handy...

    /// <summary>
    /// Robust design mode detection that works both at design time and runtime.
    /// </summary>
    private bool IsInDesignMode()
    {
        // Multiple checks for robust designer mode detection
        return LicenseManager.UsageMode == LicenseUsageMode.Designtime ||
               Site?.DesignMode == true ||
               (Site?.Container?.Components?.OfType<Control>().Any(c => c.Site?.DesignMode == true) == true);
    }```

@PWagner1
Copy link
Contributor Author

@PWagner1
And controls cant be added normally nor can they be moved across the form. https://github.com/user-attachments/assets/ecaf5425-f95d-446f-9116-dff4c0d25912

@giduac
This is pretty much of a hit and miss...
Screen.Recording.2025-09-17.084932.mp4

So disabling worked at designtime, nice.

There have been quite a few edits to try to get this fixed. Which most of those are likely unnecessary. So best is to clean all this out and get it nice and tidy. When going over the code I think there are also some stale code blocks.

After that I'd like to go over the code and have a look at the need for nullability on vars/instances to reduce null checks. A question mark doesn't look like much but will result in a full null check under the hood. So there's a bit to gain here.

# EDIT:

Or maybe better take a new branch from Alpha to lose all the changes. And add the part that turns the menu off...

And:

Since this all happens in the form instance the Form.DesignModeproperty can be used directly and don't need this code block. Several (not all) components/controls have this property handy...

    /// <summary>
    /// Robust design mode detection that works both at design time and runtime.
    /// </summary>
    private bool IsInDesignMode()
    {
        // Multiple checks for robust designer mode detection
        return LicenseManager.UsageMode == LicenseUsageMode.Designtime ||
               Site?.DesignMode == true ||
               (Site?.Container?.Components?.OfType<Control>().Any(c => c.Site?.DesignMode == true) == true);
    }```

Not necessary, as systemmenu is now disabled at design time. That's what the code block is for.

@giduac
Copy link
Contributor

giduac commented Sep 24, 2025

@PWagner1

Testing...

ShowOnAltSpace set to false still shows the themed menu.
https://github.com/user-attachments/assets/4b6ee149-7a4c-4824-9161-4a2f1233aa2f

@giduac
Copy link
Contributor

giduac commented Sep 24, 2025

@PWagner1

  • Custom menu items created in the designer
    Are not added to the menu at runtime.
    Also those are not serialised into the designer source.
    Also: add 1 item to the menu. Then remove it and try adding it again. This results in an error.

  • Also tried to mutate the menu items at run-time.
    There the items are always inserted above Close.

@PWagner1
Copy link
Contributor Author

@giduac

Please retry

@giduac
Copy link
Contributor

giduac commented Sep 25, 2025

@PWagner1

Getting close...

System menu opens on the wrong location using ALT+SPACE

TestForm_G8wLek6eq6.mp4
  • Custom menu items created in the designer, are not added to the menu at runtime.
    Those are not serialised into the designer source.

  • Problem in the designer: add 1 item to the menu. Then remove it and try adding it again. This results in an error.

devenv_8vnWDfnM55.mp4

@PWagner1
Copy link
Contributor Author

@Smurf-IV or @giduac

Would you mind taking a look at the serialisation, as a extra pair of eyes would help. I'm appearing to go round in circles.

@PWagner1
Copy link
Contributor Author

@giduac

Are we good to merge? I've opened #2530 for future improvement

@giduac
Copy link
Contributor

giduac commented Sep 29, 2025

@giduac

Are we good to merge? I've opened #2530 for future improvement

I will test first...

@giduac
Copy link
Contributor

giduac commented Sep 29, 2025

@giduac

Are we good to merge? I've opened #2530 for future improvement

OK in this state it won't pose a problem to the user and you have time to get it sorted.
One thing: In the testform page you made for the system menu there are still several references to the custom menu item which will inhibit a build within vstudio.... I just ran into that...

@PWagner1
Copy link
Contributor Author

@giduac
Are we good to merge? I've opened #2530 for future improvement

OK in this state it won't pose a problem to the user and you have time to get it sorted. One thing: In the testform page you made for the system menu there are still several references to the custom menu item which will inhibit a build within vstudio.... I just ran into that...

@giduac

Just taken out the references, so it should now build

///
/// The definitions in this file provide the foundational types and contracts that enable
/// the flexible theming, styling, and behavior customization capabilities of the Krypton Toolkit.
/// </summary>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PWagner1
cc.: @tobitege, @Smurf-IV, @KamaniAR & @lesandrog

Suppressing the warning here is imo not a good way to go about this.
The XML Doc tags have none for namespaces. Maybe this works now but could create a problem in the future.
Also the text is not about the namespace but about the content of the file.

My Suggestion is to go for the standard block comment to tell something about the file contents.
Like:

/* 
 * Core definitions file for the Krypton Toolkit containing interfaces, enums, and type definitions
 *   used throughout the Krypton UI component library.
 *   
 *   This file contains:
 *   - Core interfaces for content values, button specifications, and context menu providers
 *   - Enumerations for UI states, orientations, styles, and behaviors
 *   - Type definitions for palette states, button styles, and layout specifications
 *   - Constants and enumerations for message boxes, icons, and theme types
 *   
 *   The definitions in this file provide the foundational types and contracts that enable
 *   the flexible theming, styling, and behavior customization capabilities of the Krypton Toolkit.
 */

private bool _showOnIconClick;
private ThemedSystemMenuItemCollection? _customMenuItems;
private SystemMenuItemCollection? _customMenuItems;
#endregion
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private ThemedSystemMenuItemCollection? _customMenuItems;
  • _customMenuItems is assigned in the constructor.
  • After that the reference is never reassigned

In that case _customMenuItems does not need to be nullable.

And all these null checks are redundant.

image

// Refresh the themed system menu to reflect the new state
_themedSystemMenuService?.ThemedSystemMenu?.Refresh();
// Refresh the system menu to reflect the new state
_systemMenuService?.SystemMenu?.Refresh();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_systemMenuService?.SystemMenu?.Refresh();

SystemMenu is a non nullable property so the null check is redundant.
There multiple occurences in KForm of this,

private Rectangle _lastGripClientRect = Rectangle.Empty;
private Rectangle _lastGripWindowRect = Rectangle.Empty;
private readonly KryptonSystemMenuService? _systemMenuService;
private SystemMenuValues _systemMenuValues;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private SystemMenuValues _systemMenuValues;
Is assigned once in the constructor and is non nullable declared. After that there are no reassignements
Can be declared as read-only.

This field is null checked a lot in KForm.

image

/// <para>If detection fails or is ambiguous, returns False (assumes runtime) to ensure
/// application functionality is preserved over designer convenience.</para>
/// </remarks>
private bool IsInDesignMode() =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a condensed version of this...

image

@giduac giduac merged commit ed2ac72 into alpha Sep 30, 2025
2 checks passed
@PWagner1 PWagner1 removed their assignment Sep 30, 2025
@PWagner1 PWagner1 deleted the 648-revisited-V100 branch September 30, 2025 06:21
@PWagner1
Copy link
Contributor Author

@giduac

All of your feedback has now come up... GH WT****?

Screen.Recording.2025-09-30.075549.mp4

@giduac
Copy link
Contributor

giduac commented Sep 30, 2025

@giduac

All of your feedback has now come up... GH WT****?

Screen.Recording.2025-09-30.075549.mp4

PR meltdown.....

@tomdobing
Copy link

@PWagner1 / @giduac just for clarity - The issue of internal panel not allowing the drop of controls is this now resolved?

as i have update nightly to the latest yet i still cannot drag controls onto the form. Only way i can is by dropping them into the titlebar dragging back onto the form and then rightclicking. however it still will not show the control at run time.

Maybe i missed something i am not sure. any helpc

@PWagner1
Copy link
Contributor Author

PWagner1 commented Oct 2, 2025

@PWagner1 / @giduac just for clarity - The issue of internal panel not allowing the drop of controls is this now resolved?

as i have update nightly to the latest yet i still cannot drag controls onto the form. Only way i can is by dropping them into the titlebar dragging back onto the form and then rightclicking. however it still will not show the control at run time.

Maybe i missed something i am not sure. any helpc

Hi @tomdobing

Would you mind testing this branch? https://github.com/Krypton-Suite/Standard-Toolkit/tree/alpha-kform-internalpanel-designer-fix

cc. @giduac

@PWagner1
Copy link
Contributor Author

PWagner1 commented Oct 3, 2025

Hi @tomdobing

I can confirm that it is fixed, but .NET is the culprit for the designer bug. Please follow this workaround.

cc. @Smurf-IV & @giduac

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:designer All issues related to the designer. area:toolkit All issues related to the toolkit components. assessment:highpriority These items have been assessed as high priority, extra attention is required. version:100-lts All things to do with V100 LTS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants